-
Notifications
You must be signed in to change notification settings - Fork 30
feat: brownfield CLI #176
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: brownfield CLI #176
Conversation
…ie folder as output base
| { | ||
| "root": true, | ||
| "extends": ["@react-native", "prettier"], | ||
| "extends": ["prettier"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
intended?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, intended - otherwise CLI gets warnings due to lack of React. I'll rework the ESLint config in a separate PR anyway.
| - `gradle-plugin:lint` - runs linting on the Brownfield Gradle plugin source code | ||
| - `typecheck` - runs TypeScript type checking on all TS source files in the monorepo *[Turbo]* | ||
| - `build` - runs all `build*` tasks in the Turbo repo - see below for more details *[Turbo]* | ||
| - `build:watch` - runs all `build:watch` tasks in all workspaces |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do with just watch or dev
| @@ -0,0 +1,133 @@ | |||
| # Usage with Brownie package | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this shouldn't be a part of this PR?
| React Native Brownfield comes with a batteries-included, all-in-one CLI tool called `brownie` that helps you build and publish your React Native app as a framework artifact for iOS (XCFramework) and Android (AAR). | ||
| Additionally, it contains CLI utilities for other tools like the `@callstack/brownie` state management tool. | ||
|
|
||
| > ![NOTE] | ||
| > The `brownie` CLI is a unified tool serving both the needs of `@callstack/react-native-brownfield` and `@callstack/brownie` packages. The name `brownie` does not mean it is only meant for `@callstack/brownie` state management tool; it is the main CLI for React Native Brownfield as well. | ||
| > Technically, the CLI itself is encapsulated in the `@callstack/brownie-cli` package, which is a dependency of both `@callstack/react-native-brownfield` and `@callstack/brownie`. Therefore, installing either of the packages will bring the `brownie` CLI tool to your project. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is quite confusing that we need tell users that brownie is both CLI and a storage library :|
| "typecheck": "tsc --noEmit", | ||
| "build": "bob build && tsc -p scripts/tsconfig.json", | ||
| "build": "bob build", | ||
| "build:watch": "nodemon --watch src --ext js,ts,json --exec \"bob build\"", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: you could run tsc --watch with watch flag (what we do in Rock)
| @@ -0,0 +1,18 @@ | |||
| { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was removed in favor of change sets. I think you need a rebase
| "name": "@callstack/brownie-cli", | ||
| "version": "1.0.0", | ||
| "license": "MIT", | ||
| "author": "Michal Chudziak <[email protected]>", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
update author
| export * from './packageAndroid.js'; | ||
| export * from './packageIos.js'; | ||
| export * from './publishAndroid.js'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
avoid barrel exports please, use explicit ones
| // below: the userConfig.reactNativeVersion may be a non-semver-format string, | ||
| // e.g. '0.82' (note the missing patch component), | ||
| // therefore we resolve it manually from RN's package.json using Rock's utils | ||
| reactNativeVersion: getReactNativeVersion(projectRoot), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should be fine to pass 0.82 instead of 0.82.0
| // e.g. '0.82' (note the missing patch component), | ||
| // therefore we resolve it manually from RN's package.json using Rock's utils | ||
| reactNativeVersion: getReactNativeVersion(projectRoot), | ||
| usePrebuiltRNCore: 0, // for brownfield, it is required to build RN from source |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uh oh, this should be a boolean, not a number! we should fix that in Rock
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thymikee
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's rename the CLI to brownfield and not couple it with Brownie. Needs a rebase, otherwise it looks good!
Summary
This PR addresses a few important aspects:
The new CI builds artifacts from the RN app using the CLI and then builds the iOS & Android consumer apps using these artifacts.
Changes in this PR depend on callstackincubator/rock#666 being merged & published.
Test plan
CI green.